chore: Add a DA service unit test #3423
Open
+1,000
−745
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…/com.unity.netcode.gameobjects into chore/rust-unit-test
NoelStephensUnity
approved these changes
Apr 30, 2025
There was a problem hiding this comment.
Only 1 or 2 questions and a minor suggestion on wording for API documentation.
Otherwise, very nice job:
- Cleaning up various areas within the NetcodeIntegrationTest.
- Improved the "readability" in many places.
- Added a more standard way to obtain various NetworkManager instances.
- Cleaning up various integration tests
- Improved the "readability" in many places.
- Created a more uniform standard "flow" and naming.
- Adding UseCMBService to tests that still needed conversion along with comments.
- Removing m_EnableVerboseDebug to reduce over-all test runner log size.
🥇 💯
com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs
Outdated
Show resolved
Hide resolved
com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs
Outdated
Show resolved
Hide resolved
com.unity.netcode.gameobjects/TestHelpers/Runtime/NetcodeIntegrationTest.cs
Show resolved
Hide resolved
....netcode.gameobjects/Tests/Runtime/DistributedAuthority/NetworkClientAndPlayerObjectTests.cs
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Tests/Runtime/DistributedAuthority/OwnershipPermissionsTests.cs
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Tests/Runtime/NetworkObject/NetworkObjectOwnershipTests.cs
Show resolved
Hide resolved
com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariable/NetworkVariableCollectionsTests.cs
Show resolved
Hide resolved
testproject/Assets/Tests/Runtime/NetworkSceneManager/InScenePlacedNetworkObjectTests.cs
Show resolved
Hide resolved
This is an attempt to fix an issue where Mac OSX is frequently failing the InterpolationStopAndStartMotionTest. Based on the error, I am thinking (hard to tell since it doesn't replicate locally in-editor nor stand alone) that it could be due to a Mac VM running slower than expected (randomly) which could cause the network tick event to trigger more than once in a single frame.
michalChrobot
approved these changes
May 1, 2025
.yamato/desktop-standalone-tests.yml
Outdated
| - cd ./mps-common-multiplayer-backend/runtime && $HOME/.cargo/bin/cargo build --example ngo_echo_server | ||
| # Run the echo server in the background - this will reuse the artifacts from the build | ||
| - cd ./mps-common-multiplayer-backend/runtime && $HOME/.cargo/bin/cargo run --example ngo_echo_server -- --port $ECHO_SERVER_PORT & | ||
| - ./Tools/CI/run_cmb_service.sh |
| # Set this to ensure the DA codec tests will fail if they cannot connect to the echo-server | ||
| # The default is to ignore the codec tests if the echo-server fails to connect | ||
| ENSURE_CODEC_TESTS: "true" | ||
| USE_CMB_SERVICE: "true" |
There was a problem hiding this comment.
I don't see where we are using it 🤔
There was a problem hiding this comment.
It's inside the UseCMBService() function inside NetcodeIntegrationTest.cs (here)
.yamato/desktop-standalone-tests.yml
Outdated
| @@ -81,21 +81,17 @@ desktop_standalone_test_{{ project.name }}_{{ platform.name }}_{{ backend }}_{{ | |||
| {% if platform.name != "win" %} # Issues with win and mac are tracked in MTT-11606 | |||
| variables: | |||
| ECHO_SERVER_PORT: "7788" | |||
| CMB_SERVICE_PORT: "7799" | |||
There was a problem hiding this comment.
We could maybe add a quick comment to those 2 lines to just point where it's used/why we need 2 ports
Tools/CI/run_cmb_service.sh
Outdated
| @@ -0,0 +1,19 @@ | |||
| # clone the latest rust repo | |||
There was a problem hiding this comment.
I think that actually we could include a short "overall description" of this script (similarly as in other CI files) to describe for example
- Why we need this script (DA testing)
- Is there anything specific reader should know (example: won't work on windows)
- Is is a solution for DA only or it functions more as "distributed testing" so it could be potentially extended to different areas
- Where we can find usage of this script (desktop-standalone-tests)
There was a problem hiding this comment.
Done! Added a bunch of info at the top of the file. I hope it makes things clearer
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MTT-11550
Changelog
Testing and Documentation
Backport
No backport applicable - DA only